Prevent ConcurrentModificationException in Before notify/breadcrumb callbacks #266
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 1182
💛 - Coveralls |
kattrali
approved these changes
Mar 9, 2018
| public boolean run(Error error) { | ||
| beforeNotifyTasks.add(new BeforeNotifySkeleton()); | ||
| // modify the Set, when iterating to the next callback this should not crash | ||
| return true; |
There was a problem hiding this comment.
This test fails without the change?
There was a problem hiding this comment.
Yes, as it would modify the contents of the LinkedHashSet when an iterator has already been implicitly created in runBeforeNotifyTasks
fractalwrench
added a commit
to bugsnag/bugsnag-java
that referenced
this pull request
Aug 13, 2019
If a callback is added while existing callbacks are being iterated through, bugsnag will crash. This changeset addresses this issue by using a ConcurrentLinkedQueue which retains the callback order, but will not crash if new callbacks are added concurrently. This fix has been ported from bugsnag/bugsnag-android#266
lemnik
pushed a commit
that referenced
this pull request
Jun 2, 2021
Avoid eager initialization of bugsnag tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prevents a
ConcurrentModificationExceptionfrom occurring in theBeforeNotifyandBeforeRecordBreadcrumbcallbacks, if an element is added to the collection during or at the same time asnotifyis called.A
ConcurrentLinkedQueueis used in place of aLinkedHashSetas this retains the order of the callbacks. There's a bit of a tradeoff here with #127 - I've added a check to the client method to avoid adding duplicate callbacks, and assumed that anyone adding to the Collection directly knows what they're doing.